-
Notifications
You must be signed in to change notification settings - Fork 31
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
README: Add simple usage #125
Conversation
Rebased on top of #126 the README: Add simple usage is the commit intended for this PR |
@pevogam and if you have some time, what would you say about this (only the last commit, I keep neglecting the documentation, this could IMO help the newcomers). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @ldoktor overall a great improvement of usability for new users, I think we can just slightly improve on the advertising part and the section structure of the README.
False | ||
>>> session.is_responsive() | ||
False |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I love this idea - it is compact, goes straight to the point, and offers some of the highest information per lines read.
`sftp` and others although it can be useful for pure streams as well. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this the main advantage aexpect
has over pexpect
? Because from the first sentence it sounds almost like "this is yet another way of doing what you can already do there". It would be great to focus on what aexpect
can do and pexpect
right from the very start. I suggest a first sentence starting with "This project provides services similar to ... but enhanced them with ..." Note I used "project" instead of "module" since some of the remote code running (remote door with remote decorators, remote objects, and remote control files) and other additional functionality comes to mind as a definite advantage that I am not sure pexpect
has.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not really a marketing person so I just wanted to add that it might be useful for non-bash environments as well. Improvements to this as well as the code below (remote execution) would be definitely welcome.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not trying to request a sales pitch but merely to have one initial sentence that describes in short what the project is about and what separates it from other similar projects (why it exists on the first place). So IIUC you say there isn't much more that aexpect can do than pexpect? I thought the project was started because there was something Red Hat or others needed that pexpect did not already provide.
False | ||
|
||
Using this even for purely bash-like constructs is good as you can leverage |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is great additional information and useful to have in the documentation but if it is in the README how about separating it in a second section called "Debugging"? The first one could be called "Overview" or "Summary".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You mean just a section or a separate page? Currently the whole readme is quite short so having it in a single page seems reasonable to me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a section in markdown to separate the two parts of "Debugging" and "Overview" (the upper part).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, makes sense
aexpect/ops_windows.py
Outdated
@@ -503,6 +503,7 @@ def find_unused_port(session, start_port=10024): | |||
:returns: unused port that was found | |||
:rtype: int | |||
""" | |||
# pylint: disable=C0301 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be great to rebase this branch on top of the main branch once #126 is merged.
How about this, rebased and added some hints on extra features. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great, just a few more minor comments on typos and I think this is ready.
README.rst
Outdated
@@ -3,4 +3,78 @@ Aexpect: Control your interactive applications | |||
|
|||
This module provides services similar to the `pexpect` python library, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we phrase this as "this project"? After all, this is documentation for the entire project and aexpect is not a single module for quite a while now if since the very beginning (having more than just client
module).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ack
README.rst
Outdated
`sftp` and others although it can be useful for pure streams as well. | ||
|
||
It's enhanced of multi-pattern matching, convenience features for |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"It's enhanced with" or if you want "It also offers" / "It also has the capabilities of"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ack
README.rst
Outdated
extra output from time to time (eg. kernel messages) over the output. | ||
|
||
There are also extra classes to simplify executing parts of program |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"parts of a program"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ack
|
||
Simple usage | ||
------------ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea about this, another alternative would be "Quick start" but both work well.
let's add a very rough but practical usage guide to our README. Signed-off-by: Lukáš Doktor <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
let's add a very rough but practical usage guide to our README.